fix(LineChart): correct PointMode valueIndex and dot alignment by saadkhalidkhan · Pull Request #173 · ehsannarmani/ComposeCharts

saadkhalidkhan · 2026-06-06T17:36:27Z

Summary

Fix incorrect valueIndex in PopupProperties.Mode.PointMode for large datasets (reported with 30 data points)
Replace indexOfFirst { x >= xPosition } with nearest-neighbor lookup on pathData.xPositions to avoid off-by-one errors from floating-point rounding
Snap PointMode popups to the exact data point value and position instead of interpolating along the line
Align dot x coordinates with line path xPositions for consistent marker placement

Fixes #129

Root cause

calculateValueIndex mapped touch position using indexOfFirst { x >= xPosition } against path bounds width. Small floating-point differences pushed the index to the next data point (e.g. index 4 instead of 3). PointMode also reused interpolated popup values instead of the tapped point.

Commits

fix(LineChart): correct PointMode valueIndex with nearest data point
fix(LineChart): align dot x positions with line path xPositions

Test plan

:compose-charts:compileDebugKotlinAndroid builds successfully
:app:compileDebugKotlinAndroid builds successfully
LineChart with 30 points + PopupProperties.Mode.PointMode() — tap each dot, valueIndex matches label index
Same chart with Mode.Normal — behavior unchanged
Chart with dots enabled — markers align with line vertices on dense datasets

Summary by CodeRabbit

Bug Fixes
- Improved LineChart popups during tap and drag interactions by selecting the nearest data point more accurately.
- Corrected popup positioning and value display for single-point and point-based charts.
- Fixed dot placement so markers align consistently with plotted data points.
- Improved rendering and interaction behavior for charts containing only one data point.

Replace indexOfFirst lookup that biased toward the next index due to floating-point rounding. Find the nearest xPositions entry within the PointMode threshold and snap popups to the exact data point value.

Use the same x coordinates as the line path when drawing dots so marker positions stay consistent with large datasets and view ranges.

ehsannarmani · 2026-07-11T15:39:54Z

Hi @saadkhalidkhan, there is issue in this PR, imagine you have just one data and you enabled dots, the right thing is to see one dot in first of chart like current version:

but in your PR, we see that dot in the end of chart:

With only one value, xPositions was ending up as chart width, so the dot showed on the right. Keep it at x=0 like before.

saadkhalidkhan · 2026-07-21T09:59:05Z

Thanks for catching that.

When there was only one value, xPositions was using the chart width, so the dot ended up on the right. It should stay at the start (x = 0) like the previous behavior.

Pushed a fix for that case.

coderabbitai · 2026-07-21T09:59:05Z

📝 Walkthrough

Walkthrough

Changes

The line chart now uses consistent x-position data for single-point and multi-point series. Dot placement uses those positions directly, while popup selection resolves the nearest point and handles single-value series explicitly.

Line Chart Alignment

Layer / File(s)	Summary
Handle single-point x positions `compose-charts/src/commonMain/kotlin/ir/ehsannarmani/compose_charts/extensions/line_chart/Line.kt`	`xPositions` now records `0.0` for single-point series and the chart width for multi-point series.
Align dots and popups with x positions `compose-charts/src/commonMain/kotlin/ir/ehsannarmani/compose_charts/LineChart.kt`	Dot rendering uses supplied x positions, and popup calculations use nearest-point selection with explicit single-value and point-mode branches.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is concise and accurately summarizes the main fix to PointMode value selection and dot alignment.
Linked Issues check	✅ Passed	The changes align dots with line-path vertices and correct PointMode selection, which addresses the reported dot-position error.
Out of Scope Changes check	✅ Passed	The modified code stays focused on LineChart point selection, popup placement, and dot rendering, with no clear unrelated changes.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

🧹 Nitpick comments (1)

compose-charts/src/commonMain/kotlin/ir/ehsannarmani/compose_charts/LineChart.kt (1)
651-680: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Nearest-index logic looks correct; consider unit tests.

The bounds handling and nearest-neighbor search are correct (verified the filter { it <= lastIndex } guard protects against stale/mismatched xPositions sizes, and monotonic xPositions guarantees results stay within [startIndex, endIndex]). Given issue #129 already reported two related index/positioning bugs in this area, and the PR itself lists dense-dot/30-point manual validation as pending, adding focused unit tests for findNearestValueIndex/calculateValueIndex (single point, 2 points, dense clusters, threshold boundary) would cheaply guard against regressions here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@compose-charts/src/commonMain/kotlin/ir/ehsannarmani/compose_charts/LineChart.kt`
around lines 651 - 680, Add focused unit tests for findNearestValueIndex and
calculateValueIndex covering single-point and two-point inputs, dense point
clusters, and threshold boundary behavior. Verify nearest indices remain within
the available values range, including stale or mismatched xPositions, and
preserve the existing fallback behavior for values outside the threshold.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@compose-charts/src/commonMain/kotlin/ir/ehsannarmani/compose_charts/LineChart.kt`:
- Around line 651-680: Add focused unit tests for findNearestValueIndex and
calculateValueIndex covering single-point and two-point inputs, dense point
clusters, and threshold boundary behavior. Verify nearest indices remain within
the available values range, including stale or mismatched xPositions, and
preserve the existing fallback behavior for values outside the threshold.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f9ff9de9-108d-48d1-bcf2-e67064c221e1

📥 Commits

Reviewing files that changed from the base of the PR and between 8535289 and 04e4c73.

📒 Files selected for processing (2)

compose-charts/src/commonMain/kotlin/ir/ehsannarmani/compose_charts/LineChart.kt
compose-charts/src/commonMain/kotlin/ir/ehsannarmani/compose_charts/extensions/line_chart/Line.kt

saadkhalidkhan added 2 commits June 6, 2026 22:36

fix(LineChart): correct PointMode valueIndex with nearest data point

87e8e28

Replace indexOfFirst lookup that biased toward the next index due to floating-point rounding. Find the nearest xPositions entry within the PointMode threshold and snap popups to the exact data point value.

fix(LineChart): align dot x positions with line path xPositions

4c0ed35

Use the same x coordinates as the line path when drawing dots so marker positions stay consistent with large datasets and view ranges.

fix single data point dots staying at the start of the chart

04e4c73

With only one value, xPositions was ending up as chart width, so the dot showed on the right. Keep it at x=0 like before.

coderabbitai Bot reviewed Jul 21, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(LineChart): correct PointMode valueIndex and dot alignment#173

fix(LineChart): correct PointMode valueIndex and dot alignment#173
saadkhalidkhan wants to merge 3 commits into
ehsannarmani:masterfrom
saadkhalidkhan:fix/linechart-point-mode-value-index

saadkhalidkhan commented Jun 6, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

ehsannarmani commented Jul 11, 2026

Uh oh!

saadkhalidkhan commented Jul 21, 2026

Uh oh!

coderabbitai Bot commented Jul 21, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

saadkhalidkhan commented Jun 6, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root cause

Commits

Test plan

Summary by CodeRabbit

Uh oh!

ehsannarmani commented Jul 11, 2026

Uh oh!

saadkhalidkhan commented Jul 21, 2026

Uh oh!

coderabbitai Bot commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

saadkhalidkhan commented Jun 6, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 21, 2026 •

edited

Loading